-
-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Invert header if primary is bright and background disabled #35666
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
c8031d3
to
4368706
Compare
@@ -95,8 +95,10 @@ protected function generateGlobalBackgroundVariables(): array { | |||
// If primary as background has been request or if we have a custom primary colour | |||
// let's not define the background image | |||
if ($backgroundDeleted) { | |||
$variables['--color-background-plain'] = $this->themingDefaults->getColorPrimary(); | |||
$variables['--color-background-plain'] = $this->defaultPrimaryColor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is intentional.
We're in the admin section, we should use defaultPrimaryColor
instead of primaryColor
If logged in, this var will be overruled below in the user section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain their differences ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defaultPrimaryColor
= admin defined, instance-wide
primaryColor
= user, active for current session
6e2ee9a
to
0cf7609
Compare
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
0cf7609
to
1acd42e
Compare
Failure unrelated
|
@@ -132,14 +132,57 @@ describe('Remove the default background and restore it', function() { | |||
}) | |||
}) | |||
|
|||
describe.only('Remove the default background with a bright color', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe.only('Remove the default background with a bright color', function() { | |
describe('Remove the default background with a bright color', function() { |
🙄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And now, everybody knows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take the walk of shame 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix #35664